Log input event details to verbose log #2371

intgr · 2021-06-08T16:19:09Z

I wanted to automate a few actions using 'adb shell input' commands, but
it requires pixel coordinates. Logging input event details from scrcpy is
helpful for achieving that.

intgr · 2021-06-08T16:20:45Z

PS: Thanks a lot for developing and maintainig scrcpy. 💘

rom1v · 2021-06-09T17:03:14Z

Thank you for your contribution.

I think that the log should not be in the "convert" method (wihch is a kind of util method), but in the caller.

Input events can be very verbose, they should be enabled in verbose mode only (LOGV) IMO.

For consistency, verbose logs could also be added for other input events (keyboard, touches). (This could be done separately later though.)

intgr · 2021-06-10T08:14:23Z

👍 I will try to make changes later today.

intgr · 2021-06-10T20:41:17Z

Fixed your notes and added logging for finger events. Didn't implement keyboard event logging yet.

rom1v · 2021-06-16T20:02:57Z

After reading your LOGV calls just before a control_push_msg(), I'm wondering if we shouldn't log all control messages (with all its values).

For example, by adding a function control_msg_log(const struct control_msg *msg) called at the beginning of control_push_msg(). What do you think?

intgr · 2021-06-17T17:59:53Z

Makes sense to me.

Do you think I should log AMOTION_EVENT_ACTION_MOVE events too or just up/down? For my use case, I would find logging all events a bit noisy.

For example, if I want to script an adb shell input swipe command, I need the start and end coordinates but not the intermediate ones. But I think it isn't a big deal.

Can you imagine any use cases, where logging intermediate events would be useful?

rom1v · 2021-06-17T18:02:57Z

I would prefer to log all events sent to the device without distinction. If you want only some of them, it's easy to grep :)

intgr · 2021-06-17T19:23:12Z

@rom1v I pushed a partial draft, if you happen to be around, it would be nice to hear if I'm going in the right direction with this.

I also noticed only now (embarrassing!) that scrcpy does not currently support a SC_LOG_LEVEL_VERBOSE log level, I suppose I need to add support for that as well?

rom1v

I pushed a partial draft, if you happen to be around, it would be nice to hear if I'm going in the right direction with this.

Yes, looks good 👍 Thank you.

I added few remarks inline.

I also noticed only now (embarrassing!) that scrcpy does not currently support a SC_LOG_LEVEL_VERBOSE log level

Oops.

I suppose I need to add support for that as well?

Yes please, as a separate commit :)

app/src/android/input.h

app/src/controller.c

intgr · 2021-06-17T21:45:01Z

Thanks! My C has been getting a bit rusty. Pushed an updated version.

I added --verbosity=verbose option in a separate commit
I went through all format string arguments and should have correct types now.
Moved logging code to control_msg.c
Added remaining event types

I was trying to avoid building the Java part. Took me a while to realize that I need to downgrade, seems like it doesn't work with JDK 16 (got some weird error "module java.base does not "opens java.io" to unnamed module").

intgr · 2021-06-17T21:49:46Z

app/src/control_msg.c

+            break;
+        case CONTROL_MSG_TYPE_INJECT_TOUCH_EVENT: {
+            size_t action = msg->inject_touch_event.action & AMOTION_EVENT_ACTION_MASK;
+            LOGV("Input: touch %-4s position=%" PRIi32 "x%" PRIi32 " pressure=%g buttons=%06lx",


I omitted pointer_id from this message because it didn't didn't seem very useful.

I think you should log it, it allows to distinguish several simultaneous fingers on touch events.

server/src/main/java/com/genymobile/scrcpy/Server.java

app/src/control_msg.c

app/src/cli.c

PR #2371 <#2371> Signed-off-by: Romain Vimont <[email protected]>

rom1v · 2021-06-19T23:38:47Z

Hi,

I allowed myself to work on this based on your branch. I did the following changes:

introduce a few more macros to simplify
log pointer id (either mouse, vfinger or a numeric id)

Here is a branch: logv. Please review/test 😉

I discovered, thanks to the logs, that motion events were sent even if left-click was not pressed (but for example right-click or 4th button), so I fixed it: 0fd5622 👍

I plan to do few more things before merging:

move all log level stuff to log.h/log.c
expose a function to know the current log level
do not call control_msg_log() if the current log level is not verbose.

PR #2371 <#2371> Signed-off-by: Romain Vimont <[email protected]>

It was safe to call strcpy() since the input length was checked, but then it is more straightforward to call memcpy() directly.

Mouse motion events were forwarded as soon as any mouse button was pressed. Instead, only consider left-click (and also middle-click and right-click if --forward-all-clicks is enabled).

PR Genymobile#2371 <Genymobile#2371> Signed-off-by: Romain Vimont <[email protected]>

PR #2371 <#2371> Signed-off-by: Romain Vimont <[email protected]>

rom1v · 2021-06-20T10:56:56Z

I plan to do few more things before merging:

move all log level stuff to log.h/log.c

expose a function to know the current log level

do not call control_msg_log() if the current log level is not verbose.

Done. I updated the logv branch.

intgr · 2021-06-20T13:04:52Z

I discovered, thanks to the logs, that motion events were sent even if left-click was not pressed

Thanks, cool to hear that there are other use cases/benefits of this work :)

intgr · 2021-06-20T13:05:46Z

How do we proceed? Should I pull your changes to my branch? I'll do that for now.

rom1v · 2021-06-20T13:08:42Z

How do we proceed? Should I pull your changes to my branch?

Not necessarily, just tell me if it's ok or if some thkngs should be changed :) I'll merge manually.

app/src/controller.c

intgr · 2021-06-20T13:18:13Z

Other than that minor comment, this looks good to me.

Not necessarily, just tell me if it's ok or if some thkngs should be changed :) I'll merge manually.

OK, I already pushed it, I'm sure you can sort it out :)

app/src/control_msg.c

PR Genymobile#2371 <Genymobile#2371> Signed-off-by: Romain Vimont <[email protected]>

On Windows, PRIu64 is defined to "llu", which is not supported: error: unknown conversion type character 'l' in format

This will allow to avoid unnecessary processing for creating logs which will be discarded anyway.

If the log level is not verbose, there is no need to attept to log control messages at all.

rom1v · 2021-06-20T14:07:46Z

OK, I already pushed it, I'm sure you can sort it out :)

OK, it's indeed better that you pushed it, it's more convenient for discussions 😉

intgr · 2021-06-20T14:13:34Z

Pushed again. Seems fine to me now 👍

rom1v · 2021-06-20T14:45:53Z

Seems fine to me now +1

Thanks 👍

Then let's merge into dev 🎉

intgr changed the title ~~Log finger event coordinates in debug log~~ Log finger event coordinates to debug log Jun 8, 2021

intgr force-pushed the log-finger-event-coordinates branch 2 times, most recently from b65886d to 728f658 Compare June 10, 2021 20:39

intgr force-pushed the log-finger-event-coordinates branch 2 times, most recently from 83e824f to 3fd36c8 Compare June 17, 2021 19:20

intgr changed the title ~~Log finger event coordinates to debug log~~ Log input event details to verbose log Jun 17, 2021

rom1v reviewed Jun 17, 2021

View reviewed changes

app/src/android/input.h Outdated Show resolved Hide resolved

app/src/controller.c Outdated Show resolved Hide resolved

app/src/controller.c Outdated Show resolved Hide resolved

app/src/controller.c Outdated Show resolved Hide resolved

intgr force-pushed the log-finger-event-coordinates branch from 3fd36c8 to 2359ffd Compare June 17, 2021 21:40

intgr force-pushed the log-finger-event-coordinates branch from 2359ffd to 3d4cb1b Compare June 17, 2021 21:46

intgr commented Jun 17, 2021

View reviewed changes

server/src/main/java/com/genymobile/scrcpy/Server.java Outdated Show resolved Hide resolved

intgr commented Jun 17, 2021

View reviewed changes

app/src/control_msg.c Outdated Show resolved Hide resolved

intgr force-pushed the log-finger-event-coordinates branch from 3d4cb1b to 85200bd Compare June 17, 2021 22:01

intgr commented Jun 17, 2021

View reviewed changes

app/src/control_msg.c Show resolved Hide resolved

rom1v reviewed Jun 18, 2021

View reviewed changes

app/src/cli.c Show resolved Hide resolved

rom1v pushed a commit that referenced this pull request Jun 19, 2021

Add --verbosity=verbose log level

287da90

PR #2371 <#2371> Signed-off-by: Romain Vimont <[email protected]>

rom1v pushed a commit that referenced this pull request Jun 19, 2021

Log control messages in verbose mode

22456f1

PR #2371 <#2371> Signed-off-by: Romain Vimont <[email protected]>

rom1v pushed a commit that referenced this pull request Jun 19, 2021

Add --verbosity=verbose log level

5a1e08f

PR #2371 <#2371> Signed-off-by: Romain Vimont <[email protected]>

rom1v pushed a commit that referenced this pull request Jun 19, 2021

Log control messages in verbose mode

086564a

PR #2371 <#2371> Signed-off-by: Romain Vimont <[email protected]>

Replace strcpy() by memcpy()

df01716

It was safe to call strcpy() since the input length was checked, but then it is more straightforward to call memcpy() directly.

rom1v and others added 3 commits June 20, 2021 12:34

Remove extra ';' in #define

8b90e1d

Forward mouse motion only on main clicks

7db0189

Mouse motion events were forwarded as soon as any mouse button was pressed. Instead, only consider left-click (and also middle-click and right-click if --forward-all-clicks is enabled).

Add --verbosity=verbose log level

937fa70

PR Genymobile#2371 <Genymobile#2371> Signed-off-by: Romain Vimont <[email protected]>

rom1v pushed a commit that referenced this pull request Jun 20, 2021

Log control messages in verbose mode

0a35edd

PR #2371 <#2371> Signed-off-by: Romain Vimont <[email protected]>

intgr force-pushed the log-finger-event-coordinates branch from 85200bd to 3671e55 Compare June 20, 2021 13:08

intgr commented Jun 20, 2021

View reviewed changes

app/src/controller.c Outdated Show resolved Hide resolved

intgr commented Jun 20, 2021

View reviewed changes

app/src/control_msg.c Outdated Show resolved Hide resolved

intgr and others added 5 commits June 20, 2021 16:03

Log control messages in verbose mode

19ca02c

PR Genymobile#2371 <Genymobile#2371> Signed-off-by: Romain Vimont <[email protected]>

Workaround PRIu64 on Windows

1039f9b

On Windows, PRIu64 is defined to "llu", which is not supported: error: unknown conversion type character 'l' in format

Move log level conversion to log API

5c95d18

Expose function to get the current log level

4889911

This will allow to avoid unnecessary processing for creating logs which will be discarded anyway.

Attempt to log message only in verbose mode

0de534d

If the log level is not verbose, there is no need to attept to log control messages at all.

intgr force-pushed the log-finger-event-coordinates branch from 3671e55 to 0de534d Compare June 20, 2021 14:13

rom1v closed this Jun 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Log input event details to verbose log #2371

Log input event details to verbose log #2371

intgr commented Jun 8, 2021 •

edited

Loading

intgr commented Jun 8, 2021

rom1v commented Jun 9, 2021

intgr commented Jun 10, 2021

intgr commented Jun 10, 2021

rom1v commented Jun 16, 2021

intgr commented Jun 17, 2021

rom1v commented Jun 17, 2021

intgr commented Jun 17, 2021

rom1v left a comment

intgr commented Jun 17, 2021

intgr Jun 17, 2021

rom1v Jun 18, 2021

rom1v commented Jun 19, 2021 •

edited

Loading

rom1v commented Jun 20, 2021

intgr commented Jun 20, 2021

intgr commented Jun 20, 2021

rom1v commented Jun 20, 2021

intgr commented Jun 20, 2021

rom1v commented Jun 20, 2021

intgr commented Jun 20, 2021

rom1v commented Jun 20, 2021

Log input event details to verbose log #2371

Log input event details to verbose log #2371

Conversation

intgr commented Jun 8, 2021 • edited Loading

intgr commented Jun 8, 2021

rom1v commented Jun 9, 2021

intgr commented Jun 10, 2021

intgr commented Jun 10, 2021

rom1v commented Jun 16, 2021

intgr commented Jun 17, 2021

rom1v commented Jun 17, 2021

intgr commented Jun 17, 2021

rom1v left a comment

Choose a reason for hiding this comment

intgr commented Jun 17, 2021

intgr Jun 17, 2021

Choose a reason for hiding this comment

rom1v Jun 18, 2021

Choose a reason for hiding this comment

rom1v commented Jun 19, 2021 • edited Loading

rom1v commented Jun 20, 2021

intgr commented Jun 20, 2021

intgr commented Jun 20, 2021

rom1v commented Jun 20, 2021

intgr commented Jun 20, 2021

rom1v commented Jun 20, 2021

intgr commented Jun 20, 2021

rom1v commented Jun 20, 2021

intgr commented Jun 8, 2021 •

edited

Loading

rom1v commented Jun 19, 2021 •

edited

Loading